-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: billing #636
feat: billing #636
Conversation
garbage
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request includes significant modifications to various workflow configurations, particularly in the Android build and release processes, by removing the disk space management steps. Additionally, changes were made to several files, including updates to dependency versions in Changes
Possibly related PRs
Warning Rate limit exceeded@lovegaoshi has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
closes #600 |
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 13
🧹 Outside diff range and nitpick comments (16)
android/settings.gradle (1)
Line range hint
1-9
: Verify duplicate inclusion of React Native Gradle pluginThere appears to be duplicate inclusion of the React Native Gradle plugin:
- Line 1:
includeBuild("../node_modules/@react-native/gradle-plugin")
- Line 7:
includeBuild('../node_modules/@react-native/gradle-plugin')
This redundancy should be removed to avoid potential build conflicts.
pluginManagement { includeBuild("../node_modules/@react-native/gradle-plugin") } plugins { id("com.facebook.react.settings") } extensions.configure(com.facebook.react.ReactSettingsExtension){ ex -> ex.autolinkLibrariesFromCommand() } rootProject.name = 'azusa-player-mobile' include ':app' -includeBuild('../node_modules/@react-native/gradle-plugin') apply from: new File(["node", "--print", "require.resolve('expo/package.json')"].execute(null, rootDir).text.trim(), "../scripts/autolinking.gradle")
src/utils/Bilibili/BiliUser.ts (1)
33-43
: Improve error handling and loggingWhile the error handling is a good addition, we're silently swallowing errors which could make debugging issues difficult.
Consider adding error logging:
try { const guards = await getUserGuard(); return ( guards.filter( (guard: any) => guard.guard_level > 0 && hasGuard.includes(guard.target_id), ).length > 0 ); - } catch { + } catch (error) { + logger.error(`Failed to check guard status: ${error}`); return false; }src/components/setting/AboutSettings.tsx (2)
Line range hint
25-46
: Consider improving button labelsThe button label 'Gayhub' might be inappropriate or unclear. Consider using more professional and descriptive labels, preferably from translation keys.
- {'Gayhub'} + {t('About.GithubReleases')}
48-54
: Consider using theme values for consistent stylingThe hardcoded fontSize and padding values could be derived from theme constants for better maintainability and consistency across the app.
const styles = StyleSheet.create({ text: { - fontSize: 20, - paddingHorizontal: 20, + fontSize: theme.typography.fontSize.large, + paddingHorizontal: theme.spacing.medium, }, centeredRowContainer: { flexDirection: 'row', justifyContent: 'center' }, });src/components/billing/View.tsx (3)
19-25
: Consider adding error state handling to the LoadingWrapper.While the loading state management is clean, consider adding error state handling to provide better feedback to users when operations fail.
interface LoadingChildrenProps { setLoading: React.Dispatch<React.SetStateAction<boolean>>; + setError: React.Dispatch<React.SetStateAction<string | null>>; } const LoadingIconWrapper = ({ Child }: LoadingWrapperProps) => { const [loading, setLoading] = useState(false); + const [error, setError] = useState<string | null>(null); + if (loading) { return <ActivityIndicator />; } + if (error) { + return <Text style={{ color: 'red' }}>{error}</Text>; + } - return <Child setLoading={setLoading} />; + return <Child setLoading={setLoading} setError={setError} />; };
89-105
: Add accessibility support and improve image handling.Consider adding accessibility labels and moving the image URL to configuration.
const VIPScreen = () => { const { t } = useTranslation(); return ( - <View style={mStyle.container}> + <View + style={mStyle.container} + accessible={true} + accessibilityLabel={t('Billing.VIPScreenLabel')} + > <Image style={mStyle.azusaMock} source={{ - uri: 'https://img.nga.178.com/attachments/mon_202202/01/-zue37Q2p-6rfwK2dT1kShs-b4.jpg', + uri: IMAGES.AZUSA_MOCK, }} + accessibilityRole="image" + accessibilityLabel={t('Billing.VIPImageLabel')} />
107-131
: Name the default export component and organize styles.The default export should be a named component for better debugging and component hierarchy understanding.
-export default () => { +const BillingView = () => { const vip = useVIP(state => state.VIP); if (vip) { return <VIPScreen />; } return <PurchaseVIPScreen />; }; + +export default BillingView;src/components/style.ts (2)
Line range hint
13-42
: Consider improving type safety by reducingany
assertions.The code uses multiple
as any
type assertions which bypass TypeScript's type checking. This could lead to runtime errors that TypeScript could catch at compile time.Consider:
- Creating proper interfaces for the custom properties
- Using proper type guards instead of type assertions
- Documenting the HACK comment with a TODO for future improvement
Example refactor:
interface CustomMetaData extends NoxTheme.MetaData { // Add additional custom metadata properties } interface CustomStyle extends NoxTheme.Style { metaData: CustomMetaData; customColors: Record<string, string>; // Add other custom properties } export const createStyle = ( customStyle: CustomStyle | NoxTheme.AdaptiveStyle = AzusaTheme, ) => { const refTheme = customStyle.metaData.darkTheme ? NoxTheme : AzusaTheme; return StyleSheet.create({ metaData: { ...refTheme.metaData, // TODO: Improve type safety of metadata merging ...customStyle.metaData, }, // ... rest of the implementation }); };
Line range hint
89-98
: Enhance error handling for invalid colors.The current implementation logs errors but continues with invalid colors, which could lead to runtime visual issues.
Consider throwing an error or returning a safe fallback color:
export const replaceStyleColor = ({ playerStyle, primaryColor = playerStyle.colors.primary, // ... other params }: ReplaceStyleColor) => { const colors = [primaryColor, secondaryColor, contrastColor, backgroundColor]; if (!validateColors(colors)) { logger.error('[color converter] Invalid colors:', colors); // Either throw an error throw new Error('Invalid color values provided'); // Or use fallback colors return { ...playerStyle, colors: { ...playerStyle.colors, // Keep original colors as fallback } }; } // ... rest of the implementation };src/components/explore/SongTab.tsx (3)
8-8
: Consider optimizing style importsThe imported
styles
from '../style' is only used for theflex
style. Consider moving this style to the local StyleSheet or creating a shared styles file for commonly used styles.Also applies to: 19-19
Line range hint
44-66
: Consider extracting remaining inline stylesWhile the move to StyleSheet is good, there are still inline styles that could be extracted:
- Text styles with dynamic colors
- Padding and flex styles
Consider this approach for better style organization:
// In the StyleSheet textTitle: { fontSize: 20, paddingLeft: 5, flex: 1, }, textSubtitle: { paddingLeft: 5, }, // In the component <Text style={[ style.textTitle, { color: fontColor } ]} >
170-183
: Improve responsive design handlingThe current implementation has potential responsiveness issues:
- Direct usage of
Dimensions.get('window').width
might not update on orientation changes- Hardcoded height values might not work well across different screen sizes
Consider these improvements:
- Use the
useWindowDimensions
hook for responsive dimensions that update with orientation changes- Use relative units or flex for height calculations
- Consider implementing a responsive design system
Example implementation:
import { useWindowDimensions } from 'react-native'; // Inside component const { width } = useWindowDimensions(); // In StyleSheet cardContainer: { width: width * 0.8, flex: 1, // Instead of fixed height maxHeight: 390, // Optional maximum height paddingHorizontal: 5, paddingRight: 10, },src/components/setting/View.tsx (1)
99-104
: Consider optimal placement for premium features.While the implementation is correct, consider moving the premium settings item higher in the list for better visibility and to emphasize its importance. Common patterns place premium/subscription options near the top of settings menus for better discovery.
@@ -45,6 +45,11 @@ ]} > <ScrollView> + <SettingListItem + icon={NoxView.PREMIUM} + settingName="PremiumSetting" + onPress={() => navigation.navigate(NoxView.PREMIUM)} + settingCategory="Settings" + /> <SettingListItem icon={NoxView.HOME} settingName="GeneralSetting"package.json (1)
127-127
: Consider updating metro resolutionThe metro resolution is maintained while other resolutions were removed. Consider if this resolution is still necessary or if it can be removed for consistency.
src/localization/en/translation.json (1)
213-214
: Enhance premium settings descriptionThe description could be more informative about the available premium features.
Consider this improvement:
"PremiumSettingName": "Premium APM", - "PremiumSettingDesc": "Buy Premium features", + "PremiumSettingDesc": "Upgrade to Premium for an ad-free experience and exclusive features",src/hooks/useVIP.ts (1)
68-70
: Add dependency touseEffect
hook for clarityWhile the empty dependency array in
useEffect
ensures theinit
function runs once on mount, it's good practice to include all external dependencies to avoid potential issues in the future.Modify the
useEffect
to includeinit
as a dependency:useEffect(() => { init(); -}, []); +}, [init]);If
init
is defined within the component, this change ensures consistency and clarity, especially ifinit
relies on props or state that may change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
.yarn/patches/@react-native-gradle-plugin-npm-0.76.1-f19c439d00.patch
is excluded by!**/.yarn/**
.yarn/patches/react-native-npm-0.76.1-c738a2fa39.patch
is excluded by!**/.yarn/**
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (19)
.github/workflows/android-weekly.yml
(0 hunks).github/workflows/play-release.yml
(0 hunks)android/gradle.properties
(1 hunks)android/settings.gradle
(1 hunks)package.json
(5 hunks)patches/@sharcoux+slider+8.0.4.patch
(0 hunks)patches/react-native-tab-view+4.0.1.patch
(0 hunks)src/App.tsx
(2 hunks)src/components/billing/View.tsx
(1 hunks)src/components/billing/bill.ts
(0 hunks)src/components/explore/SongTab.tsx
(9 hunks)src/components/setting/AboutSettings.tsx
(3 hunks)src/components/setting/DeveloperSettings.tsx
(0 hunks)src/components/setting/View.tsx
(4 hunks)src/components/style.ts
(1 hunks)src/hooks/useVIP.ts
(1 hunks)src/localization/en/translation.json
(2 hunks)src/localization/zhcn/translation.json
(3 hunks)src/utils/Bilibili/BiliUser.ts
(1 hunks)
💤 Files with no reviewable changes (6)
- .github/workflows/android-weekly.yml
- .github/workflows/play-release.yml
- patches/@Sharcoux+slider+8.0.4.patch
- patches/react-native-tab-view+4.0.1.patch
- src/components/billing/bill.ts
- src/components/setting/DeveloperSettings.tsx
🧰 Additional context used
🪛 eslint
src/components/billing/View.tsx
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
src/components/setting/AboutSettings.tsx
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
🔇 Additional comments (22)
android/settings.gradle (2)
10-10
: LGTM: Expo modules configuration
The useExpoModules()
call is correctly placed after the Expo autolinking script is applied.
Line range hint 1-10
: Verify build configuration after dependency substitution removal
The removal of dependency substitution block (as mentioned in the summary) could affect how React Native modules are resolved during build time. Please verify:
- The app builds successfully without the substitutions
- All React Native modules are properly resolved from node_modules
- The change aligns with the React Native version being used
✅ Verification successful
Let me check the specific React Native configurations in those Gradle files to verify if the current setup aligns with React Native 0.76.1.
The current Gradle configuration is correct and aligned with React Native 0.76.x
The current setup in settings.gradle
is using the modern React Native Gradle plugin configuration:
- Properly includes the Gradle plugin from
@react-native/gradle-plugin
- Uses the new settings plugin
com.facebook.react.settings
- Configures autolinking through
ReactSettingsExtension
- No dependency substitutions are present, which is correct for RN 0.76.x
The removal of dependency substitutions is actually the recommended approach for React Native 0.76.x, as the new Gradle plugin handles module resolution automatically.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check React Native version and related configurations
echo "Checking React Native version..."
if [ -f package.json ]; then
jq '.dependencies["react-native"]' package.json
echo "Checking for any custom React Native configurations..."
# Look for any custom React Native configurations in gradle files
find android -name "*.gradle" -type f -exec grep -l "react-native" {} \;
fi
Length of output: 425
Script:
#!/bin/bash
# Check contents of identified Gradle files
echo "=== android/app/build.gradle ==="
grep -A 10 "react-native" android/app/build.gradle
echo -e "\n=== android/build.gradle ==="
grep -A 10 "react-native" android/build.gradle
echo -e "\n=== android/settings.gradle ==="
cat android/settings.gradle
Length of output: 3818
src/utils/Bilibili/BiliUser.ts (1)
33-43
: Verify the impact on billing features
Since this function is part of the billing implementation and the related bill.ts
file was removed, we should verify its integration with the new billing system.
src/components/setting/AboutSettings.tsx (2)
1-1
: LGTM! Good practice using StyleSheet.
The addition of StyleSheet import and migration from inline styles to StyleSheet.create follows React Native best practices for better performance and maintainability.
🧰 Tools
🪛 eslint
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
20-24
: Verify if billing-related disclaimers are needed
Since this PR implements billing features, consider whether any payment, subscription, or premium feature disclaimers should be added to this About section.
Consider adding a separate BillingSettings component for payment-related information if it doesn't already exist.
android/gradle.properties (1)
38-38
: Architectural Change: Disabling New React Native Architecture
This change disables the new React Native architecture (TurboModules and Fabric renderer). While this might resolve immediate issues, it's a significant architectural regression that could impact performance and future feature adoption.
Could you please:
- Explain why this change was necessary for the billing implementation
- Document this decision in an ADR (Architecture Decision Record)
- Create a tracking issue for re-enabling the new architecture once the blocking issues are resolved
Let's check if any native modules in the project require the new architecture:
src/App.tsx (3)
55-58
: Review splash screen duration logic
The splash screen duration now varies based on three conditions:
- Development mode (
__DEV__
) - Intent data presence (
appProps.intentData
) - VIP status (
vip
)
This creates different user experiences:
- 1ms for dev/intent/VIP users
- 2500ms for others
Consider if this significant difference in loading times (2.5s) provides a good user experience for non-VIP users.
#!/bin/bash
# Check for other splash screen duration references
rg "useSplash.*[0-9]+" -A 2
Line range hint 59-59
: Verify VIP-dependent player features
The vip
status is passed to useSetupPlayer
, suggesting player functionality might vary based on VIP status. Ensure this integration is properly documented and tested.
#!/bin/bash
# Check how useSetupPlayer handles the vip prop
ast-grep --pattern 'useSetupPlayer = ({ $_, vip }) => {
$$$
}'
19-19
: Verify VIP initialization timing
The switch from useVIP
to useSetupVIP
suggests this is now handling VIP feature initialization. Ensure that this initialization doesn't block the app's startup unnecessarily.
✅ Verification successful
No startup performance concerns with VIP initialization
The useSetupVIP
hook implementation shows that:
- VIP initialization is non-blocking and runs asynchronously
- It only configures purchases on Android and performs a VIP check
- The splash screen duration is properly adjusted based on VIP status (2500ms for non-VIP, 1ms for VIP users)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the implementation of useSetupVIP to understand initialization timing
ast-grep --pattern 'export const useSetupVIP = () => {
$$$
}'
Length of output: 223
Script:
#!/bin/bash
# Let's try finding the hook implementation with ripgrep
rg -l "useSetupVIP" --type ts --type tsx
# Then examine the content of any matching files
rg "useSetupVIP" -A 10 --type ts --type tsx
Length of output: 140
Script:
#!/bin/bash
# Search for the hook implementation without file type restrictions
rg -l "useSetupVIP"
# Then examine the content of files containing the hook
rg "useSetupVIP" -A 10
Length of output: 1661
src/components/billing/View.tsx (1)
1-18
: LGTM! Well-structured imports and type definitions.
The imports are appropriate and the TypeScript interfaces are well-defined.
🧰 Tools
🪛 eslint
[error] 1-1: Resolve error: Cannot find module 'babel-plugin-module-resolver'
Require stack:
- /home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js
- /home/jailuser/git/node_modules/eslint-module-utils/resolve.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/rules/no-unresolved.js
- /home/jailuser/git/node_modules/eslint-plugin-import/lib/index.js
- /home/jailuser/git/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
at Module._load (node:internal/modules/cjs/loader:1074:27)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
at Module.require (node:internal/modules/cjs/loader:1339:12)
at require (node:internal/modules/helpers:135:16)
at Object. (/home/jailuser/git/node_modules/eslint-import-resolver-babel-module/lib/index.js:8:5)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
at Module.load (node:internal/modules/cjs/loader:1317:32)
(import/namespace)
src/components/style.ts (1)
151-151
: LGTM: New centerText style follows React Native conventions.
The addition of the centerText
style property is clean and follows React Native's StyleSheet patterns.
src/components/explore/SongTab.tsx (1)
Line range hint 52-64
: Review setTimeout usage and add error handling
The code contains a HACK comment with setTimeout which might indicate a potential race condition or timing issue with the scroll functionality.
Let's search for similar patterns in the codebase:
Additionally, consider adding proper error handling and loading states:
onPress={async () => {
try {
await playAsSearchList({
songs: totalSongs ?? songs,
song: item,
});
await new Promise(resolve => setTimeout(resolve, 500));
scroll();
} catch (error) {
// Handle error appropriately
console.error('Failed to play song:', error);
}
}}
✅ Verification successful
Let me gather more context about the scroll
function and playAsSearchList
to better understand the timing dependencies.
Let me check the implementation of useNoxSetting
and incSongListScrollCounter
to understand the scroll state management better.
The setTimeout usage is a valid implementation for scroll synchronization
The code pattern is intentional and serves a specific purpose:
- The
scroll
function is actuallyincSongListScrollCounter
from the app state store that triggers a scroll effect inSongList.tsx
- The 500ms delay allows the playlist state to update and the player navigation to complete before scrolling
- This pattern is consistently used across multiple components (
SongRow.tsx
,SongTab.tsx
) to ensure proper synchronization - The scroll effect is properly handled in
SongList.tsx
with auseEffect
hook that responds to the counter changes
The HACK comment can be replaced with a more descriptive explanation of the timing requirement, but the implementation itself is sound and doesn't indicate a race condition.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for setTimeout usage with scroll-related functionality
rg -A 3 "setTimeout.*scroll"
Length of output: 840
Script:
#!/bin/bash
# Search for scroll function definition and usage
ast-grep --pattern 'const scroll = $_'
# Search for playAsSearchList implementation
ast-grep --pattern 'const playAsSearchList = $_'
# Search for any comments or patterns related to scroll timing
rg -B 2 -A 2 'scroll.*500'
Length of output: 2965
Script:
#!/bin/bash
# Search for useNoxSetting implementation
ast-grep --pattern 'const useNoxSetting = $_'
# Search for incSongListScrollCounter definition
ast-grep --pattern 'incSongListScrollCounter: $_'
# Search for any related state updates
rg "songListScrollCounter" -B 2 -A 2
Length of output: 15469
src/components/setting/View.tsx (3)
16-17
: LGTM! Consistent naming convention.
The renamed import LoginSettings
and new import PremiumSettings
follow the established naming pattern of other settings components.
31-31
: LGTM! Clear icon mapping.
The PREMIUM
enum with 'cash' icon is appropriately placed and follows the existing pattern.
179-183
: Verify translation key exists.
The implementation looks good, but let's verify that the translation key appDrawer.PremiumName
exists in the localization files.
✅ Verification successful
Translation key is correctly implemented
The translation key appDrawer.PremiumName
exists in both English and Chinese localization files under the correct namespace, with appropriate translations ("Premium Content" in English and "高级播放器" in Chinese).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the translation key exists in localization files
# Search for the translation key in all translation files
rg -l "PremiumName" --type json
Length of output: 107
Script:
#!/bin/bash
# Check the actual content of translation files to verify the key structure
rg "PremiumName" --json src/localization/en/translation.json src/localization/zhcn/translation.json
Length of output: 1575
Script:
#!/bin/bash
# Check if the translation key is under the correct 'appDrawer' namespace
rg -A 5 -B 5 '"appDrawer"' src/localization/en/translation.json src/localization/zhcn/translation.json
Length of output: 1674
package.json (3)
Line range hint 104-104
: Verify billing implementation dependencies
I notice react-native-purchases
is present but not modified. For the billing feature implementation:
- Confirm if version 8.2.6 supports all required billing features
- Consider documenting any specific billing provider requirements
#!/bin/bash
# Check for billing-related configuration files
fd -g "*billing*" -g "*purchase*" || echo "No billing configuration files found"
# Search for billing-related code
rg -l "react-native-purchases|RevenueCat|StoreKit|Billing" || echo "No billing implementation found"
48-48
: Verify security implications of dependency updates
Several dependencies have been updated:
- @sharcoux/slider: 8.0.4 -> 8.0.5
- expo: ^52.0.2 -> ^52.0.4
- react-native-svg: 15.8.0 -> 15.9.0
Please review the changelogs for security patches and breaking changes.
Also applies to: 64-64, 110-110
✅ Verification successful
Let me gather more information about the changelogs and recent updates for these packages.
Let me analyze the dependency update from @sharcoux/slider: 8.0.4 -> 8.0.5
. I'll check the changelog and any potential security implications.
Let me try checking the npm registry directly for information about this package update.
Based on the information gathered, I can now provide a final assessment of the @sharcoux/slider
update from 8.0.4 to 8.0.5:
The dependency update appears to be safe to proceed with
- No security vulnerabilities were found for this package
- The package maintains its core functionality as a lightweight slider for React Native
- The update appears to be a minor patch version change
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Fetch latest versions and security advisories
echo "Checking @sharcoux/slider..."
curl -s "https://registry.npmjs.org/@sharcoux/slider" | jq '.versions["8.0.5"].dist.integrity'
echo "Checking expo..."
curl -s "https://registry.npmjs.org/expo" | jq '.versions["52.0.4"].dist.integrity'
echo "Checking react-native-svg..."
curl -s "https://registry.npmjs.org/react-native-svg" | jq '.versions["15.9.0"].dist.integrity'
# Check for known vulnerabilities
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "expo") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}'
Length of output: 1217
Script:
#!/bin/bash
# Check changelogs and release information
echo "Checking @sharcoux/slider changelog..."
curl -s "https://raw.githubusercontent.com/Sharcoux/slider/master/CHANGELOG.md" || \
curl -s "https://api.github.com/repos/Sharcoux/slider/releases/tags/v8.0.5"
echo -e "\nChecking expo changelog..."
curl -s "https://raw.githubusercontent.com/expo/expo/main/CHANGELOG.md" || \
curl -s "https://api.github.com/repos/expo/expo/releases/tags/sdk-52.0.4"
echo -e "\nChecking react-native-svg changelog..."
curl -s "https://raw.githubusercontent.com/software-mansion/react-native-svg/master/CHANGELOG.md" || \
curl -s "https://api.github.com/repos/software-mansion/react-native-svg/releases/tags/v15.9.0"
# Check for any security advisories for the other packages
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "react-native-svg") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}'
Length of output: 326097
Script:
#!/bin/bash
# Check changelog and release info for @sharcoux/slider
echo "Checking @sharcoux/slider release info..."
curl -s "https://api.github.com/repos/Sharcoux/slider/releases/tags/v8.0.5"
echo -e "\nChecking @sharcoux/slider commits between versions..."
curl -s "https://api.github.com/repos/Sharcoux/slider/compare/v8.0.4...v8.0.5"
Length of output: 656
Script:
#!/bin/bash
# Check npm registry for package information
echo "Checking @sharcoux/slider npm package info..."
curl -s "https://registry.npmjs.org/@sharcoux/slider" | jq '.versions["8.0.5"].dist.integrity, .versions["8.0.5"].time, .versions["8.0.5"].description'
# Check for any known vulnerabilities
echo -e "\nChecking for vulnerabilities..."
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "@sharcoux/slider") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}'
Length of output: 816
40-40
: Verify the impact of removing patch files for React Native dependencies
The transition from patch files to direct versions for @react-native/gradle-plugin
and react-native
could affect build stability. Please ensure:
- The patches weren't addressing critical issues specific to your implementation
- All functionality previously modified by patches still works as expected
Also applies to: 89-89
✅ Verification successful
The previous script had syntax errors and didn't provide clear information. Let's try with these commands to:
- Find any existing patch files
- Check the history of package.json for patch-related changes
- Look for gradle configuration files that might be affected by the version change
Transition from patches to direct versions is safe to proceed
Based on the git history and current patch files, I can confirm:
- The removed patches for
react-native
and@react-native/gradle-plugin
were temporary fixes for the new architecture transition, which are now incorporated into the main versions. - Other critical patches remain in place (e.g., @react-native-cookies/cookies, react-native-blob-util, etc.) showing selective patch management.
- The change was part of a deliberate "chore: remove newarch patch" commit, indicating it was a planned cleanup after stabilization.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining patch files and their contents
fd -e patch -x cat {} \; || echo "No patch files found"
# List all files modified in the last commit that removed the patches
git log -p --all -S "patch:react-native" --pretty=format: -G "patch:" | grep "^+" || echo "No patch-related changes found"
Length of output: 608
Script:
#!/bin/bash
# Check for patch files in the repository
fd -e patch
# Check package.json history for patch-related changes
git log -p package.json | grep -A 5 -B 5 "patch"
# Look for any gradle-plugin related configurations
fd -e gradle -e properties
Length of output: 24141
src/localization/zhcn/translation.json (2)
28-29
: LGTM: Consistent premium feature naming.
The translation for premium features in the app drawer is consistent with other premium-related strings in the application.
227-228
: LGTM: Consistent settings translations.
The premium settings translations are clear and consistent with other premium-related strings in the application.
src/localization/en/translation.json (2)
16-17
: LGTM: Clear and consistent labeling
The "Premium Content" label in the app drawer is clear and follows the existing naming pattern.
Line range hint 1-400
: Verify translation consistency with Chinese localization
Ensure that all new billing-related strings have corresponding translations in the Chinese localization file (src/localization/zhcn/translation.json
).
✅ Verification successful
Based on the script results, I can see that all billing-related keys (NoxAuth
, NoxFans
, PremiumFeaturesIntro
, RevenueCat
, godBlessU
, thankU
, urVeryVeryGorgeous
) exist in both English and Chinese translation files. Let me verify the actual content of these translations to ensure completeness.
All billing translations are properly synchronized between English and Chinese
The verification shows that:
- All billing-related keys exist in both language files
- Each key has appropriate translations that maintain the same meaning while being culturally adapted
- No missing or inconsistent translations found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all billing-related keys exist in both English and Chinese translation files
# Expected: All keys should be present in both files
# Extract billing-related keys from English translations
echo "Checking English billing keys..."
jq -r '.Billing | keys[]' src/localization/en/translation.json
echo "Checking Chinese billing keys..."
jq -r '.Billing | keys[]' src/localization/zhcn/translation.json
# Compare premium-related keys
echo "Comparing premium-related keys..."
for file in src/localization/{en,zhcn}/translation.json; do
echo "File: $file"
jq -r '. | to_entries[] | select(.key | test("Premium|Billing")) | .key' "$file"
done
Length of output: 1078
Script:
#!/bin/bash
# Compare the actual content of billing translations
echo "English Billing translations:"
jq '.Billing' src/localization/en/translation.json
echo -e "\nChinese Billing translations:"
jq '.Billing' src/localization/zhcn/translation.json
Length of output: 1132
(guard: any) => | ||
guard.guard_level > 0 && hasGuard.includes(guard.target_id), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add type safety for guard object
The any
type for the guard parameter could lead to runtime errors if the API response structure changes.
Consider defining an interface for the guard object:
interface Guard {
guard_level: number;
target_id: number;
}
Then update the filter:
- (guard: any) =>
+ (guard: Guard) =>
src/components/billing/View.tsx
Outdated
const BiliVIP = ({ setLoading }: LoadingChildrenProps) => { | ||
const { t } = useTranslation(); | ||
|
||
const checkBiliVIP = async () => { | ||
setLoading(true); | ||
await checkGuardVIP(); | ||
setLoading(false); | ||
}; | ||
|
||
return <Button onPress={checkBiliVIP}>{t('Billing.NoxAuth')}</Button>; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling and user feedback for VIP check.
The current implementation lacks error handling and doesn't provide feedback to users about the success or failure of the VIP check operation.
const BiliVIP = ({ setLoading }: LoadingChildrenProps) => {
const { t } = useTranslation();
const checkBiliVIP = async () => {
setLoading(true);
+ try {
- await checkGuardVIP();
+ const result = await checkGuardVIP();
+ if (result) {
+ // Show success message
+ }
+ } catch (error) {
+ logger.error('BiliVIP check failed:', error);
+ // Show error message to user
+ } finally {
setLoading(false);
+ }
};
return <Button onPress={checkBiliVIP}>{t('Billing.NoxAuth')}</Button>;
};
Committable suggestion skipped: line range outside the PR's diff.
src/components/billing/View.tsx
Outdated
const PurchaseVIPScreen = () => { | ||
const { t } = useTranslation(); | ||
return ( | ||
<View style={mStyle.container}> | ||
<Image | ||
style={mStyle.azusaBeg} | ||
source={{ | ||
uri: 'https://img.nga.178.com/attachments/mon_202201/31/-zue37Q2p-ixpkXsZ7tT3cS9y-af.gif', | ||
}} | ||
/> | ||
<Text>{t('Billing.PremiumFeaturesIntro')}</Text> | ||
<View style={styles.alignMiddle}> | ||
<LoadingIconWrapper Child={BiliVIP} /> | ||
<LoadingIconWrapper Child={RevenueCatVIP} /> | ||
</View> | ||
<Text>{t('Billing.NoxFans')}</Text> | ||
</View> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Move hardcoded URLs to configuration and add image error handling.
The hardcoded image URL should be moved to a configuration file, and error handling should be added for image loading.
+// Add to a config file
+export const IMAGES = {
+ AZUSA_BEG: 'https://img.nga.178.com/attachments/mon_202201/31/-zue37Q2p-ixpkXsZ7tT3cS9y-af.gif'
+};
+
const PurchaseVIPScreen = () => {
const { t } = useTranslation();
+ const [imageError, setImageError] = useState(false);
+
return (
<View style={mStyle.container}>
<Image
style={mStyle.azusaBeg}
source={{
- uri: 'https://img.nga.178.com/attachments/mon_202201/31/-zue37Q2p-ixpkXsZ7tT3cS9y-af.gif',
+ uri: IMAGES.AZUSA_BEG,
}}
+ onError={() => setImageError(true)}
/>
+ {imageError && (
+ <Text style={styles.errorText}>{t('Common.ImageLoadError')}</Text>
+ )}
Committable suggestion skipped: line range outside the PR's diff.
src/components/billing/View.tsx
Outdated
const RevenueCatVIP = ({ setLoading }: LoadingChildrenProps) => { | ||
const { t } = useTranslation(); | ||
const checkRevenueCatVIP = async () => { | ||
setLoading(true); | ||
try { | ||
const offerings = await Purchases.getOfferings(); | ||
if ( | ||
offerings.current !== null && | ||
offerings.current.availablePackages.length !== 0 | ||
) { | ||
const { customerInfo } = await Purchases.purchasePackage( | ||
offerings.current.availablePackages[0], | ||
); | ||
if ( | ||
typeof customerInfo.entitlements.active['apm-pro'] !== 'undefined' | ||
) { | ||
purchaseVIP(); | ||
} | ||
} | ||
} catch (e) { | ||
logger.error(JSON.stringify(e)); | ||
} | ||
setLoading(false); | ||
}; | ||
|
||
return ( | ||
<Button onPress={checkRevenueCatVIP}>{t('Billing.RevenueCat')}</Button> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve RevenueCat purchase flow implementation.
Several improvements can be made to enhance code quality and user experience:
- Extract magic strings to constants
- Simplify nested conditions
- Add user feedback for errors
+const VIP_ENTITLEMENT_ID = 'apm-pro';
+
const RevenueCatVIP = ({ setLoading }: LoadingChildrenProps) => {
const { t } = useTranslation();
+
+ const hasAvailableOfferings = (offerings: Purchases.Offerings) =>
+ offerings.current?.availablePackages.length > 0;
+
const checkRevenueCatVIP = async () => {
setLoading(true);
try {
const offerings = await Purchases.getOfferings();
- if (
- offerings.current !== null &&
- offerings.current.availablePackages.length !== 0
- ) {
- const { customerInfo } = await Purchases.purchasePackage(
- offerings.current.availablePackages[0],
- );
- if (
- typeof customerInfo.entitlements.active['apm-pro'] !== 'undefined'
- ) {
- purchaseVIP();
- }
+ if (!hasAvailableOfferings(offerings)) {
+ throw new Error(t('Billing.NoOfferings'));
}
+
+ const { customerInfo } = await Purchases.purchasePackage(
+ offerings.current!.availablePackages[0],
+ );
+
+ if (customerInfo.entitlements.active[VIP_ENTITLEMENT_ID]) {
+ await purchaseVIP();
+ // Show success message
+ }
} catch (e) {
logger.error(JSON.stringify(e));
+ // Show error message to user
+ Alert.alert(t('Billing.PurchaseError'), e.message);
+ } finally {
+ setLoading(false);
}
- setLoading(false);
};
Committable suggestion skipped: line range outside the PR's diff.
src/components/explore/SongTab.tsx
Outdated
source={{ uri: item.cover }} | ||
/> | ||
<View style={{ flex: 1 }}> | ||
<Image style={style.cardThumbnail} source={{ uri: item.cover }} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for image loading
The Image component lacks error handling which could lead to a poor user experience if the cover image fails to load.
Add error handling and a placeholder:
<Image
style={style.cardThumbnail}
source={{ uri: item.cover }}
placeholder={require('@assets/default-cover.png')}
onError={(error) => {
console.warn('Failed to load cover image:', error);
}}
/>
src/hooks/useVIP.ts
Outdated
const vipStatus = SecureStore.getItem(VIPKey); | ||
switch (vipStatus) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correctly handle asynchronous SecureStore.getItemAsync
in checkVIP
The SecureStore.getItemAsync
method is asynchronous and returns a Promise. Currently, it's being used synchronously, which will lead to incorrect behavior. Ensure that you await the Promise to retrieve the VIP status correctly.
Modify the code to await the asynchronous call:
-export const checkVIP = async () => {
- const vipStatus = SecureStore.getItem(VIPKey);
+export const checkVIP = async () => {
+ const vipStatus = await SecureStore.getItemAsync(VIPKey);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const vipStatus = SecureStore.getItem(VIPKey); | |
switch (vipStatus) { | |
const vipStatus = await SecureStore.getItemAsync(VIPKey); | |
switch (vipStatus) { |
src/hooks/useVIP.ts
Outdated
const loseVIP = () => { | ||
SecureStore.deleteItemAsync(VIPKey); | ||
useVIP.setState({ VIP: false }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Await SecureStore.deleteItemAsync
in loseVIP
to ensure data is deleted
Similarly, SecureStore.deleteItemAsync
is asynchronous and should be awaited to ensure the VIP status is properly removed before updating the state.
Modify the function as follows:
-const loseVIP = () => {
+const loseVIP = async () => {
await SecureStore.deleteItemAsync(VIPKey);
useVIP.setState({ VIP: false });
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const loseVIP = () => { | |
SecureStore.deleteItemAsync(VIPKey); | |
useVIP.setState({ VIP: false }); | |
const loseVIP = async () => { | |
await SecureStore.deleteItemAsync(VIPKey); | |
useVIP.setState({ VIP: false }); |
src/hooks/useVIP.ts
Outdated
VIP: SecureStore.getItem(VIPKey) !== null, | ||
})); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Initialize VIP state correctly by handling asynchronous retrieval
SecureStore.getItemAsync
is asynchronous, but it's being used synchronously in the state initialization. This can cause the initial VIP state to be incorrect.
Consider initializing VIP to false
and then updating the state after fetching the value:
-const useVIP = create<VIPStore>((set, get) => ({
- VIP: SecureStore.getItem(VIPKey) !== null,
+const useVIP = create<VIPStore>(() => ({
+ VIP: false,
}));
+// Initialize VIP state after fetching from SecureStore
+const initializeVIP = async () => {
+ const vipStatus = await SecureStore.getItemAsync(VIPKey);
+ useVIP.setState({ VIP: vipStatus !== null });
+};
+initializeVIP();
Alternatively, you can perform this initialization within a useEffect
hook in your component.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
VIP: SecureStore.getItem(VIPKey) !== null, | |
})); | |
const useVIP = create<VIPStore>(() => ({ | |
VIP: false, | |
})); | |
// Initialize VIP state after fetching from SecureStore | |
const initializeVIP = async () => { | |
const vipStatus = await SecureStore.getItemAsync(VIPKey); | |
useVIP.setState({ VIP: vipStatus !== null }); | |
}; | |
initializeVIP(); |
src/hooks/useVIP.ts
Outdated
export const checkGuardVIP = async () => { | ||
if (await getHasGuard([529249, 7706705])) { | ||
return purchaseVIP(); | ||
} | ||
return loseVIP(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid hardcoding IDs in checkGuardVIP
; use configuration instead
The hardcoded IDs [529249, 7706705]
in checkGuardVIP
reduce flexibility and make maintenance difficult. Consider injecting these IDs through configuration or environment variables to allow for easier updates and scalability.
Apply this refactor to pass IDs as parameters:
-export const checkGuardVIP = async () => {
- if (await getHasGuard([529249, 7706705])) {
+export const checkGuardVIP = async (guardIds: number[]) => {
+ if (await getHasGuard(guardIds)) {
return purchaseVIP();
}
return loseVIP();
};
Update calls to checkGuardVIP
accordingly by passing the appropriate IDs.
Committable suggestion skipped: line range outside the PR's diff.
src/hooks/useVIP.ts
Outdated
return; | ||
} | ||
if (isAndroid) { | ||
Purchases.configure({ apiKey: 'goog_XXAuAgmqFMypeJIGHTlyRZNdoGh' }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid hardcoding API keys; use environment variables or secure storage
Hardcoding the API key 'goog_XXAuAgmqFMypeJIGHTlyRZNdoGh'
poses a security risk and can lead to unauthorized access if the codebase is exposed. It's best practice to store sensitive information like API keys securely.
Replace the hardcoded key with an environment variable:
-import { APPSTORE } from '@env';
+import { APPSTORE, PURCHASES_API_KEY } from '@env';
...
if (isAndroid) {
- Purchases.configure({ apiKey: 'goog_XXAuAgmqFMypeJIGHTlyRZNdoGh' });
+ Purchases.configure({ apiKey: PURCHASES_API_KEY });
}
Ensure that PURCHASES_API_KEY
is defined in your environment configuration and not checked into version control.
Committable suggestion skipped: line range outside the PR's diff.
closes #623
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Chores
package.json
for improved performance and stability.